<!DOCTYPE html>
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="stylesheet" href="https://best.openssf.org/assets/css/style.css">
<link rel="stylesheet" href="checker.css">
<script src="checker.js"></script>
<script src="shell-injection.js"></script>
<link rel="license" href="https://creativecommons.org/licenses/by/4.0/">

<!-- See create_labs.md for how to create your own lab! -->

</head>
<body>
<!-- For GitHub Pages formatting: -->
<div class="container-lg px-3 my-5 markdown-body">
<h1>Lab Exercise shell-injection</h1>
<p>
This is a lab exercise on developing secure software.
For more information, see the <a href="introduction.html" target="_blank">introduction to
the labs</a>.

<p>
<h2>Task</h2>
<p>
<b>Eliminate a shell injection vulnerability.</b>

<p>
<h2>Background</h2>
<p>
In many cases, programmers run system commands from their applications.
This happens for multiple reasons: running a complicated tool, avoiding to
code a functionality that exists in another tool,
or just because the application
(often a web application) is an interface to a tool running
on the server system.
Doing this incorrectly, however, can lead to vulnerabilities.
</p>

<p>
Quite often the list of parameters of that external tool isn't static.
One or multiple parameters are given by an untrusted end user.
This may be the name
they give to the project, for example.
The application needs to avoid passing these parameters through the shell
at all if the shell isn't needed.
</p>

<p>
In addition,
the application need to carefully handle
the given parameters to make sure they do not contain special characters like
the end of the command (often ';'). Otherwise, an attacker might be
able to craft input
in such a way that they execute commands of their choice in addition to the
one present in the application.
We need to do that with an allowlist, that is, by expressly listing what
is allowed (and forbidding everything else).
</p>

<p>
<h2>Task Information</h2>
<p>
In this task we are going to fix a shell command injection situation. You may
want to test the result of different user input separately to see what happens.
</p>

<p>
Our scenario is a simple one. In our application in Python, we want to list
files in a temporary directory used by our application for data processing.
The original code was:
</p>

<pre><code>  subprocess.run("ls -l", shell=True)</code></pre>

<p>
Then, another developer added user input for the directory with the raw user
data in the variable <tt>dir_to_list</tt> as follows:
</p>

<pre><code>def list_directory(dir_to_list):
    subprocess.run(f"ls -l {dir_to_list}", shell=True)
</pre></code>

<p>
This is a shell injection vulnerability.
An attacker can put arbitrary text into the parameter, which will be
executed by the shell.
It would safer to ensure
that only safe characters would be considered, <i>and</i> to
<i>not</i> invoke the shell at all.
By taking both steps, we make it much harder for an attacker
to find a way to work around it.
</p>

<p>
We can offer a few hints.
You'll need to modify a string to only contain alphanumerics.
Python's <tt>re.sub(PATTERN, REPLACETEXT, ORIGINAL_VALUE)</tt>
returns the result of substituting
<tt>PATTERN</tt> with
<tt>REPLACETEXT</tt> in <tt>ORIGINAL_VALUE</tt>, where <tt>PATTERN</tt>
is a regular expression written as a string.
By default <tt>re.sub</tt> substitutes all matches (that's what you want).
In Python regular expressions are usually expressed
using the "raw" string form, that is, using the <tt>r'...'</tt> syntax.
You need to stop invoking the shell in Python's
<tt>subprocess.run</tt>, including separating the arguments.
</p>

<p>
To be fair, a typical Python program wouldn't call "ls" directly.
We'll do that to keep the example simple, since there are times when
you need to invoke a shell.
</p>

<p>
Use the “hint” and “give up” buttons if necessary.
</p>

<p>
<h2>Interactive Lab (<span id="grade"></span>)</h2>
<p>
Rewrite the following function to be safe:
<p>
<form id="lab">
<pre><code>def list_directory(dir_to_list):
  # Modify the directory so it only contains a-zA-Z0-9
<textarea id="attempt0" rows="2" cols="60" spellcheck="false">
  clean_dir = dir_to_list
</textarea>
  # Then use subprocess in a safer way
<textarea id="attempt1" rows="2" cols="60" spellcheck="false">
  subprocess.run(f"ls -l {dir_to_list}", shell=True)
</textarea>
</code></pre>
<button type="button" class="hintButton">Hint</button>
<button type="button" class="resetButton">Reset</button>
<button type="button" class="giveUpButton">Give up</button>
<br><br>
<p>
<i>This lab was developed by Marta Rybczynska.</i>
<br><br>
<p>
<p id="correctStamp" class="small">
<textarea id="debugData" class="displayNone" rows="20" cols="65" readonly>
</textarea>
</form>
</div><!-- End GitHub pages formatting -->
</body>
</html>
